Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes #1274 Prevent crash of 0 simulated values in log scale #1275

Merged
merged 3 commits into from
Sep 6, 2024
Merged

Fixes #1274 Prevent crash of 0 simulated values in log scale #1275

merged 3 commits into from
Sep 6, 2024

Conversation

pchelle
Copy link
Collaborator

@pchelle pchelle commented Sep 6, 2024

Provides a warning for such a case:

! Warning	Plot scale is logarithmic, however all values from simulated output 'Organism|PeripheralVenousBlood|Dextrorphan-O-glucuronide|Plasma (Peripheral Venous Blood)' were 0.
Configuration Plan Information:
SectionId: 301
Title: Quinidine-Dextromethorphan-DDI
SimulationDuration: 60
TimeUnit: h
OutputMappings: list(...)
PlotNumber: 1 (json numbering starting at 0);  2 (actual plot index)

@pchelle
Fixes Open-Systems-Pharmacology#1274 Prevent crash of 0 simulated val…
4111752 
…ues in log scale

Provides a warning for such a case
@pchelle pchelle marked this pull request as ready for review September 6, 2024 13:41
@pchelle
Copy link
Collaborator Author

pchelle commented Sep 6, 2024

Resulting plot

comparison_time_profile_Quinidine_Dextromethorphan_DDI_2

Yuri05
Yuri05 approved these changes Sep 6, 2024
View reviewed changes
R/qualification-comparison-time-profile.R Outdated
@@ -124,7 +124,14 @@ addOutputToComparisonTimeProfile <- function(outputMapping, simulationDuration,
molWeight = molWeight
)
simulatedValues <- simulatedValues[selectedTimeValues]

logScaleIssue <- all(simulatedValues == 0, isIncluded(axesProperties$y$scale, "log"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's check for <=0

R/qualification-comparison-time-profile.R Outdated
warning(messages$warningLogScaleIssue(outputMapping$Output), call. = FALSE)
# Set the first value to 1 to avoid log scale issue
# Since this only affect one value, no line will plotted
simulatedValues[1] <- 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this. Can't we just don't add the curve to the log plot at all in this case?

@Yuri05 Yuri05 merged commit 7444997 into Open-Systems-Pharmacology:develop Sep 6, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yuri05 Yuri05 Yuri05 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pchelle @Yuri05